Skip to content

credential gate: block message still claims a guaranteed audit log (#119 follow-up) - #224

Closed
firaen22 wants to merge 3 commits into
F-e-u-e-r:mainfrom
firaen22:fix/override-log-best-effort
Closed

credential gate: block message still claims a guaranteed audit log (#119 follow-up)#224
firaen22 wants to merge 3 commits into
F-e-u-e-r:mainfrom
firaen22:fix/override-log-best-effort

Conversation

@firaen22

@firaen22 firaen22 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

What

#222 corrected both READMEs: the credential-gate override's audit log is best-effort, not guaranteed. The same overclaim is still live in the hook's own block message.

hooks/gate-credential-destruction.py:224, printed to the agent at block time:

re-run prefixed with CRED_GATE_APPROVED=1 (the override is logged).

_log (lines 108-115) is try: … except Exception: pass — a failed write is dropped silently and never blocks. So the one string an agent actually reads when deciding whether to override still promises the guarantee #222 retracted, and it outranks the README for that decision: nobody consults the README mid-block.

This is the same defect class already tracked in-repo as threat-model I6 (skills-staging/2026-07-30-security-enhancement/UNCERTAINTY.md:111, and skill-vetting-security-invariants §148: "threat-model I6 said a failed write 'is logged' and both READMEs called advisory…"). #222 closed the README half; this closes the hook half.

Change

One string, matching #222's wording:

re-run prefixed with CRED_GATE_APPROVED=1 (the override attempts an audit log entry, best-effort — a failed write is dropped silently and never blocks).

Verified

  • ast.parse clean.
  • Block path: rm ~/.ssh/id_rsa → exit 2, new text printed.
  • Override path: CRED_GATE_APPROVED=1 rm ~/.ssh/id_rsa → exit 0. Unchanged.
  • python3 .github/checks.py — all checks passed.

Not changed

hooks/verify-before-stop.py:20 says "the pass is logged for audit" but its own header already qualifies logging as best-effort four lines down, so it does not carry the same defect. Left alone rather than widened into this change.

Second commit — the same overclaim in the docstring

Found while stacking a later change: fixing only BLOCK_MESSAGE left the identical guarantee in the module docstring, so this PR did not close its own defect class on the first pass.

  • line 35 — "Overridden hits are logged." → "attempt an audit log entry (best-effort — see _log)".
  • line 70 — "Audit events append to ~/.claude/hooks/hooks.log." → states that _log swallows every write error, so a failed or unwritable log is dropped silently and never blocks; the log is friction and telemetry, not a guaranteed audit trail.

Left alone: line 43's "with the traceback logged", which describes the same best-effort _log and is now covered by the corrected line 70.

Re-verified after the second commit: ast.parse clean, block path exit 2, override path exit 0, checks.py all passed.

Third commit — dual review of the second

Both families reviewed the docstring commit; both returned the same MAJOR, and it says my earlier judgment call was wrong.

  • Line 43 was an independent overclaim after all. I had left "fails open with the traceback logged" reasoning that the corrected line 70 covered it. It does not: a later sentence does not retroactively qualify an earlier one, and the Behavior section is precisely what a session reads when the hook fails open — so a failed-open bug plus a dropped log would look, from the docstring, like a traceback was left behind. Now: "fails open with a traceback audit entry attempted on a best-effort basis (see _log)".
  • "write error" was narrower than the code. _log wraps os.makedirs too and catches Exception, so a non-write failure is swallowed the same way. Line 70 now says it swallows every exception, directory creation included.

Re-verified after this commit: ast.parse clean, block path exit 2, override path exit 0, checks.py passed, and grep -n 'logged\b' over the file now returns nothing — no unqualified logging guarantee remains.

@F-e-u-e-r

Copy link
Copy Markdown
Owner

Landed via owner-curated replacement — closing without merge (this repo's consolidation convention).

Your current-head wording (9c06888) is live on main via #228 (merge commit 5824f30), landed verbatim — declared adaptations NONE, machine-verified against your frozen diff. All three sites now state the settled best-effort contract: overridden hits attempt an audit entry; the fail-open traceback entry is attempted best-effort; the module contract says _log swallows every exception (directory creation included), a failed or unwritable log drops silently, and the log is friction and telemetry, not a guaranteed audit trail.

The mechanical gate proved "text corrected, runtime unchanged": AST identical modulo string constants, _log() byte-identical, block/override exit codes unchanged, hook suites green, zero unqualified guaranteed-logging wording remaining, verify-before-stop.py untouched. #119 remains correctly CLOSED — this was the same defect family's residual on the runtime-facing surfaces the README fix never scoped, exactly as your PR framed it. Your third commit's self-corrections (the standalone traceback overclaim; _log swallowing every exception, not just write errors) both landed as-is.

Thank you — catching that the message an agent reads at block time still overpromised, after the docs were already fixed, is precisely the residual-surface discipline this pack's own doctrine now teaches.

@F-e-u-e-r F-e-u-e-r closed this Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants